-
-
Couldn't load subscription status.
- Fork 20
fix: clear state in useImage hook
#44
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
| // clear state each time we will load a new image | ||
| if (image.image || image.error) { | ||
| setImage({ image: undefined, error: undefined }); | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
At first I put it in the cleanup function but I moved this, as setting new state from it is not safe. We check if state is "dirty " to skip this in "initial" call
|
I guess with #41 we could check if source is same and skip it in that case const isSameSource = (a: AsyncImageSource, b: AsyncImageSource) => {
if (!b) return false;
if (isHybridObject(a) && isHybridObject(b)) {
return a.equals(b);
}
return JSON.stringify(a) === JSON.stringify(b);
};
const shouldClearState = (source: AsyncImageSource, result: Result) => {
const { image, error } = result;
// If there was an error, we need to clear the state
if (error) return true;
// If there is an image, we check if the source has changed
// if not, we don't need to clear the state
if (image && "__source" in image && image.__source) {
// @ts-expect-error - We save the source on the JS side so we can diff it
return !isSameSource(source, image.__source);
}
return false;
}
...
// in hook
if (shouldClearState(source, image)) {
setImage({ image: undefined, error: undefined });
}But this can lead to unpredictable behavior - eg. what if one endpoint returns random image? imo this should be handled be cache that users can control but let know what do you think about this |
1692a58 to
8a0ca27
Compare
|
hey - is this PR ready as is? |
8a0ca27 to
cfbd545
Compare
|
Hey! It seems I forgot to commit my changes 😅 - now it's ready! |
Summary
Clear state in
useImagehook, each time when loading new image.As documented in file, we should go back to "loading state" while we are loading new source
fixes #37